Skip to content

Conversation

rafaelha
Copy link
Contributor

@rafaelha rafaelha commented Oct 7, 2025

This pull request adds support for correlated qubit loss noise channel to

  • the squin.noise dialect and associated stdlib
  • stim dialect
  • PyQrack interpreter

@rafaelha rafaelha requested a review from johnzl-777 October 7, 2025 14:13
@johnzl-777 johnzl-777 changed the title Bloqade qec 58/correlated loss support correlated loss support Oct 7, 2025
Copy link

codecov bot commented Oct 7, 2025

Codecov Report

❌ Patch coverage is 89.58333% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/bloqade/stim/rewrite/squin_noise.py 86.36% 3 Missing ⚠️
src/bloqade/squin/stdlib/broadcast/noise.py 66.66% 1 Missing ⚠️
src/bloqade/squin/stdlib/simple/noise.py 75.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Contributor

github-actions bot commented Oct 7, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
9437 8340 88% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/bloqade/pyqrack/device.py 80% 🟢
src/bloqade/pyqrack/squin/noise/native.py 100% 🟢
src/bloqade/squin/_init_.py 100% 🟢
src/bloqade/squin/noise/_interface.py 100% 🟢
src/bloqade/squin/noise/stmts.py 100% 🟢
src/bloqade/squin/stdlib/broadcast/_init_.py 100% 🟢
src/bloqade/squin/stdlib/broadcast/noise.py 74% 🟢
src/bloqade/squin/stdlib/simple/_init_.py 100% 🟢
src/bloqade/squin/stdlib/simple/noise.py 74% 🟢
src/bloqade/stim/_init_.py 100% 🟢
src/bloqade/stim/_wrappers.py 100% 🟢
src/bloqade/stim/dialects/noise/emit.py 100% 🟢
src/bloqade/stim/dialects/noise/stmts.py 100% 🟢
src/bloqade/stim/rewrite/squin_noise.py 93% 🟢
TOTAL 94% 🟢

updated for commit: 73ea0b4 by action🐍

For python 3.10, cirq 1.5.0 is used since later versions only support 3.11. Here, the test suite raises 10k warning which all originate from within cirq.
@rafaelha
Copy link
Contributor Author

rafaelha commented Oct 7, 2025

Coverage is failing but that's due to the skipped files (due to ongoing refactor).

A test in bloqade-circuit/test/stim/passes/squin_noise_to_stim.py still needs to be added, but it might be best to wait until that file is refactored.

EDIT: coverage resolved


[tool.pytest.ini_options]
testpaths = "test/"
filterwarnings = [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The python 3.10 build is producing a lot of warnings (this currently also happens on main)

test/cirq_utils/test_parallelize.py: 9170 warnings
test/qasm2/test_native.py: 1873 warnings
  /Users/rafaelhaenel/Documents/quera/kirin-workspace/.venv/lib/python3.10/site-packages/cirq/circuits/circuit_operation.py:173: FutureWarning: In cirq 1.6 the default value of `use_repetition_ids` will change to
  `use_repetition_ids=False`. To make this warning go away, please pass
  explicit `use_repetition_ids`, e.g., to preserve current behavior, use
  
    CircuitOperations(..., use_repetition_ids=True)
    warnings.warn(msg, FutureWarning)

These warnings are coming internally from cirq 1.5.0. The solution is to simply upgrade to cirq 1.6 which requires Python 3.11 (which is why only the 3.10 build has these issues).

I've gone ahead and filtered these warnings.

@rafaelha rafaelha requested a review from kaihsin October 7, 2025 16:17
p (float): Probability of the qubits being lost.
qubits (IList[Qubit, Any]): The list of qubits to which the correlated noise channel is applied.
"""
broadcast.correlated_qubit_loss(p, qubits)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhat hacky. correlated_qubit_loss is an n-qubit operator. As far as I know all other operators act on either one or two qubits.

Stim does not support n-qubit operators, which is why the nonce workaround is introduced.

I'm wondering if broadcast should be disallowed for this type of operator? Instead only use apply? I am a bit confused how that works after the refactor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mismatch in semantics is a concern. There are a couple stim gates that act on more qubits: MPP, SPP, and SPP_DAG

# Perform a SQRT_YY gate between qubit 1 and 2, and a SQRT_ZZ_DAG between qubit 3 and 4.
SPP Y1*Y2 !Z1*Z2

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the requirements here? Do we really need to support correlated loss for more than two qubits at a time? If so, it's probably best to make this statement apply to an IList[IList[Qubit]] or similar. All statements in the gate dialect should "broadcast", meaning that it applies to each entry in the list of arguments.

@cduck are you saying we need to support statements for these gates? Or are you just pointing out that these would be points where this noise process can be injected and so we have more than two qubits here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the example @cduck sketched out for me I think it should be an arbitrary number of atoms.

From Casey himself:

What I mean is
I_ERROR[CORRELATED_LOSS:<nonce>](p) 1 2 3
Is there is a chance p that a single event that looses all atoms 1, 2, and 3 occurs.

Briefly chatted with @david-pl , IList[IList[Qubit]] makes sense here.

In the apply case you just accept a single list of qubits (if one atom in the list goes, all the other ones get lost too)

In the broadcast case you accept list of lists, if any atom in one of those sublists goes, then the other atoms should go too

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes in the case of SPP is why we have discussion whether we need PauliString Type or something to express a pauli-string. iirc the conclusion is to just use string "XYXXYZ" to represent SPP in stim dialect, and no support for it in squin?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to have an equivalent of the SPP command in kirin-stim at all. This is a really niche gate that we don't need. I was just using that as an example of a more complex stim instruction than I_ERROR.


@cduck are you saying we need to support statements for these gates? Or are you just pointing out that these would be points where this noise process can be injected and so we have more than two qubits here?

@david-pl, I'm throwing out ideas for stim gates that can be repurposed to represent correlated atom loss. SPP isn't a great option but shows how stim can group qubits.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cduck ok so thats align with what we discuss earlier with @ChenZhao44 that we don't really have need of SPP. For the tag. If thats the behavior of stim, then, is the quest here that you want to also preserve the command order in stim circuit python instance (i.e. don't want stim to auto merge? so we need separate tags?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

broadcast.correlated_qubit_noise has now been updated to accept qubits: list[list[Qubit]]

image

@rafaelha rafaelha requested review from cduck and david-pl October 15, 2025 00:10
Copy link
Collaborator

@david-pl david-pl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just some minor comments.

Copy link
Contributor

@johnzl-777 johnzl-777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! @david-pl brings up pretty much all the necessary changes and I just have a concern on the use of a random default number for nonce generation

Copy link
Contributor

@johnzl-777 johnzl-777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rafaelha rafaelha merged commit ff4d74f into main Oct 16, 2025
11 checks passed
@rafaelha rafaelha deleted the bloqade-qec-58/correlated-loss-support branch October 16, 2025 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants